Repository navigation
Keep stacked workspace cards one per row after device rotation - #105353
Conversation
… so they stay one per row after rotation Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🤖 Web test steps verification results: On web, More features and Accounting (Connections) look correct at wide and narrow widths, and they switch layouts correctly when you resize back and forth. I couldn't check the JS console because the web test tool can't read it, though nothing on screen showed an error. The PR's Tests section is still a TODO, so I tested the layout behavior the PR description covers.
The rotation bug this PR fixes only happens in the native Android app, so this web run checks for regressions only. It doesn't test the fix itself. |
|
@MelvinBot add QA Steps |
|
I added QA Steps to the PR description.
I didn't run these steps for this update. My earlier Android run couldn't rotate the emulator, so the rotation fix still needs a manual check on a device. |
|
@MelvinBot pull main |
|
@codex review |
|
@MelvinBot review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Review: code looks good, no blocking issuesThe fix matches the root cause. Before merge, someone still needs to fill in the Tests section and add screenshots, including an Android rotation video. Root cause confirmed in Yoga. React Native 0.86's Yoga computes an explicit Layout should be unchanged:
Still needed before merge:
|
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
The failing check is unrelated to this PR, so I didn't push a fix. Analysis: Evidence: The PR changes Next step: Rerun the full Reassure Performance Tests workflow. Rerunning only the failed job reuses the same measurements, so it won't help. |
|
🤖 Web test steps verification results: ✅ pass I found no layout regressions on More features or Accounting (Connections) at 375px, 900px, or 1440px. Cards stay full width with even margins, nothing clips or scrolls sideways, and toggling a feature switch leaves the layout unchanged.
I also reviewed the code for native regressions and found none. view run · no recording available |
|
@MelvinBot pull main |
|
The failing Analysis: The Evidence
To clear it, re-run the failed |
|
@MelvinBot pull main |
|
The failing check is unrelated to this PR, so I didn't push a code change. Analysis:
Evidence: The PR changes Next step: Rerun the failed jobs in the Reassure Performance Tests run. |
…rdFlexBasis Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
I merged The merge had no conflicts. None of the new commits from |
|
The failing checks are unrelated to this PR, so I didn't push a fix.
A new Reassure run on the latest commit is still in progress. If it fails again on Evidence
|
|
@MelvinBot pull main to fix flaky test |
…rdFlexBasis Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
@situchan perf tests are failing, can you take a look? |
|
@MelvinBot pull main |
…rdFlexBasis Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Fixed
Unrelated |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@lakchote looks like this was merged without a test passing. These checks had not passed on 4d52fe7 when it was merged:
Please add a note explaining why this was done and remove the |
|
🚧 lakchote has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.5.28-4 🚀
|
|
No help site update needed. This PR is a layout bug fix that keeps workspace feature and accounting cards one per row after you rotate a device, so no help site article sentence becomes incorrect. |
|
Not an emergency. The Android build failed to download a Mapbox dependency from repo.grails.org, which isn't related to this PR. The same job passed on main. |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.5.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.5.30-0 🚀
Bundle Size Analysis (Sentry): |
















Explanation of Change
On native, the cards on Workspace > More features and Workspace > Accounting (Connections) stack one per row. Each card used
flexBasis: '100%'to fill the row. Native Yoga caches a percentageflexBasisacross rotations. Right after you rotate from portrait to landscape, the cards keep their portrait width, so two cards fit side by side. Reopening the page already shows one card per row, so the column count was inconsistent between the two.This PR replaces
styles.flexBasis100with[styles.flexBasisAuto, styles.w100]inMoreFeatureToggle,ConnectionCard, andExactOnlineWaitlistCard. A percentagewidthis recomputed on every layout. The onboarding tiles already use this approach for the same reason (seeonboardingAccountingItemWideinsrc/styles/index.ts).The wide (multi-column) layout is unchanged, because these styles only apply in the narrow/stacked layout.
AI tests run by MelvinBot:
npm run typecheck,npm run lint-changed,npm run spell-changed, andreact-compiler-compliance-checkon the changed files all passed. The related Jest suites (WorkspaceMoreFeaturesPageTest,ExactOnlineConnectionListingTest,WorkspaceConnectionsPageAutoStartFlowTest) passed (48 tests). The rotation fix still needs manual verification on an Android device. MelvinBot's web session failed to load, so no UI check ran.Fixed Issues
$ #105148
PROPOSAL: #105148 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
QA Steps
Precondition: Set the device font size to the minimum. Enable the
unifiedConnectionsbeta. Use a workspace with Accounting enabled in More features.Android: Native and iOS: Native (fix verification)
Web and mWeb (regression)
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari